Skip to content

[release-3.4] add test release makefile target #19855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

joshjms
Copy link
Member

@joshjms joshjms commented May 4, 2025

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

Add test-release Makefile target for release-3.4 Prow job.

ref: kubernetes/test-infra#32754

/cc @ivanvc @jmhbnz

joshjms added 2 commits May 3, 2025 14:17
This is a cherrypick of 64cd6b6
PR: etcd-io#19815

Signed-off-by: joshjms <[email protected]>
Address reviews in etcd-io#19815

This is a cherrypick of c940bf5
PR: etcd-io#19848

Signed-off-by: joshjms <[email protected]>
@joshjms joshjms force-pushed the release-3.4-add-test-release-makefile-target branch 3 times, most recently from dd672c9 to fbe3511 Compare May 4, 2025 09:46
@joshjms
Copy link
Member Author

joshjms commented May 4, 2025

@ivanvc please let me know if the manual cherrypick procedures are followed and if there's anything I should do. thanks!

@joshjms
Copy link
Member Author

joshjms commented May 4, 2025

/retest

@ivanvc
Copy link
Member

ivanvc commented May 5, 2025

/retitle [release-3.4] add test release makefile target

@k8s-ci-robot k8s-ci-robot changed the title [release 3.4] add test release makefile target [release-3.4] add test release makefile target May 5, 2025
Copy link
Member

@ivanvc ivanvc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request, @joshjms. Please take a look at my comments :)

Comment on lines +56 to +78
function determine_go_version {
# Borrowing from how Kubernetes does this:
# https://github.com/kubernetes/kubernetes/blob/17854f0e0a153b06f9d0db096e2cd8ab2fa89c11/hack/lib/golang.sh#L510-L520
#
# default GO_VERSION to content of .go-version
GO_VERSION="${GO_VERSION:-"$(cat "./.go-version")"}"
if [ "${GOTOOLCHAIN:-auto}" != 'auto' ]; then
# no-op, just respect GOTOOLCHAIN
:
elif [ -n "${FORCE_HOST_GO:-}" ]; then
export GOTOOLCHAIN='local'
else
GOTOOLCHAIN="go${GO_VERSION}"
export GOTOOLCHAIN
fi
}

determine_go_version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a clean cherry pick of this commit (it's missing the documentation before the function definition)?

And the same note about the co-author trailer :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep only the last bullet point from the commit message? It's the part being backported from that commit.

Also, your co-authored trailer is incorrect. It should be Co-authored-by. Refer to https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line

@joshjms
Copy link
Member Author

joshjms commented May 5, 2025

Thanks for the review Ivan, will fix immediately. Done!

@joshjms joshjms force-pushed the release-3.4-add-test-release-makefile-target branch 2 times, most recently from e85a01c to e43d8fa Compare May 6, 2025 00:10
Partial backport of etcd-io#19305

Refactor scripts/test_images.sh
* Rename the input "IMAGE" to "TEST_IMAGE" to avoid clashing with the
  environment variable from the Prow infrastructure.

Co-authored-by: ivanvc <[email protected]>
Signed-off-by: joshjms <[email protected]>
@joshjms joshjms force-pushed the release-3.4-add-test-release-makefile-target branch from e43d8fa to d1f110d Compare May 6, 2025 00:11
Additionally, provide ability to opt-out of the .go-version and use a
custom one via env vars: FORCE_HOST_GO and GO_VERSION.

Co-authored-by: Madhav Jivrajani <[email protected]>
Signed-off-by: joshjms <[email protected]>
@joshjms joshjms force-pushed the release-3.4-add-test-release-makefile-target branch from d1f110d to 013d5b4 Compare May 6, 2025 00:13
@joshjms
Copy link
Member Author

joshjms commented May 6, 2025

/retest

Copy link
Member

@ivanvc ivanvc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, @joshjms. I think we should aim to fully backport #19305 to minimize disparity, but this can be done in a follow-up pull request.

@joshjms
Copy link
Member Author

joshjms commented May 6, 2025

Definitely, I'm planning to do it. Thanks!

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, ivanvc, joshjms

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ahrtr ahrtr merged commit 8f95cf2 into etcd-io:release-3.4 May 7, 2025
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants